-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
T345056: dark mode support #110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good! Just a few code hygiene comments inline.
src/init.js
Outdated
init( getColorScheme() ); | ||
|
||
// Re-init as needed as the color scheme changes | ||
observeDarkModePluginActivation( ( scheme ) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could further simplify by passing the function directly instead of creating a lambda with the same signature as the function.
src/link/colorScheme.js
Outdated
const attNameWPDarkModePlugin = 'data-wp-dark-mode-active'; | ||
|
||
const isWPDarkModePluginActive = () => { | ||
const htmlTag = document.documentElement; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No real use for the htmlTag variable
src/link/colorScheme.js
Outdated
} | ||
} ); | ||
|
||
observer.observe( htmlTag, { attributes: true } ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No real use for the htmlTag variable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
https://phabricator.wikimedia.org/T345056